-
Notifications
You must be signed in to change notification settings - Fork 0
Prepare for using SQLite on AWS (backup on S3) #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds SQLite backup functionality to S3, enabling the use of SQLite as a database for AWS AppRunner deployments without data loss on restarts. The changes eliminate the need for RDS, making the infrastructure more cost-effective.
Changes:
- Implemented automated SQLite database backup to S3 with hourly backups and backup on shutdown
- Migrated from
fastapi_utils.tasks.repeat_everyto FastAPI's built-in lifespan management for background tasks - Refactored authentication classes into a separate module for better code organization
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| workerfacing_api/settings.py | Added cron interval configurations for timeout checks and backups |
| workerfacing_api/main.py | Replaced repeat_every decorator with lifespan context manager and added backup cron job |
| workerfacing_api/dependencies.py | Extracted auth classes, added S3 client setup, and implemented conditional queue instantiation (SQLite vs RDS) |
| workerfacing_api/core/queue.py | Added SQLiteRDSJobQueue class with backup/restore functionality, refactored RDSJobQueue with lazy engine initialization |
| workerfacing_api/core/auth.py | New file containing extracted authentication classes (APIKeyDependency, GroupClaims, WorkerGroupCognitoCurrentUser) |
| tests/unit/core/test_queue.py | Updated test to work with simplified create() signature |
| tests/integration/test_main.py | Added comprehensive tests for timeout handling and backup/restore functionality |
| tests/integration/endpoints/test_jobs_post.py | Updated fixture scope from function to default |
| tests/integration/endpoints/test_jobs.py | Refactored tests to use isinstance checks instead of env string comparisons |
| tests/integration/endpoints/test_files.py | Refactored tests to use isinstance checks and updated fixture scopes |
| tests/integration/endpoints/conftest.py | Added client fixture with proper lifespan context manager |
| tests/integration/conftest.py | Restructured test fixtures to support parametrized filesystem and queue combinations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage Report
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@nolan1999 I've opened a new pull request, #82, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@copilot i get an error in the following test: The pre-signed URLs for posting work fine on the main branch, so something is wrong in this branch. Note how in the code, some specific stuff is done in the |
|
@nolan1999 I've opened a new pull request, #83, to work on those changes. Once the pull request is ready, I'll request review from you. |
Support backup to S3 of local SQLite DB. This allows restarting
AppRunnerwithout losing all the data if using SQLite as a database. This will make the stack cheaper, since no RDS is required anymore.